Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#6 - timestamp playlist inserts when adding to a live showEndTime #25

Merged

Conversation

eric-gilbertson
Copy link
Collaborator

add timestamp to track inserts on playlist if show is realtime, eg
insert is done during the user specified begin/end times. also simplify
markup generation by breaking large functions up and by using CSS to
reduce the amount of required markup. other changes include:

  • show time in 12 hour format
  • show album & artist names as 'first last' (was 'last, first')
  • remove the butt ugly heart logo
  • simplify playlist banner
  • add playlist CSS classes
  • normailize border width
  • add border between playlist header and body
  • improved track edit glyphs
  • replace " with ' for readability
  • update zkdbSchema.sql and provide an upgrade script
  • protect against empty arrays in for loops (generate ugly warning messages)
  • make album link clickable in playlist edit mode (now matches view mode markup)
  • added 'created' to tracks query (as last column in the select statement)
  • added function to check if insert is within show start/end times

…dTime

add timestamp to track inserts on playlist if show is realtime, eg
insert is done during the user specified begin/end times. also simplify
markup generation by breaking large functions up and by using CSS to
reduce the amount of required markup. other changes include:

  - show time in 12 hour format
  - show album & artist names as 'first last' (was 'last, first')
  - remove the butt ugly heart logo
  - simplify playlist banner
  - add playlist CSS classes
  - normailize border width
  - add border between playlist header and body
  - improved track edit glyphs
  - replace \" with ' for readability
  - update zkdbSchema.sql and provide an upgrade script
  - protect against empty arrays in for loops (generate ugly warning messages)
  - make album link clickable in playlist edit mode (now matches view mode markup)
  - added 'created' to tracks query (as last column in the select statement)
  - added function to check if insert is within show start/end times
@RocketMan RocketMan merged commit cc892c4 into RocketMan:master May 20, 2019
@RocketMan
Copy link
Owner

Hello Eric,

Thank you, all of the issues named previous have been addressed; I have merged the PR.

As promised, I do have a couple functional comments for your consideration. None of these needs to be addressed before we deploy (if ever), but I did want to share:

  • As a DJ moves spins up or down in the playlist editor, can zk really know the specific times of those spins anymore? As it stands now, the timestamp does not move with the spin, which seems correct... one would not want to move the timestamp, as the times in the playlist would then be out of temporal order. However, one other solution would be to clear the timestamps of the two tracks involved the move, since moving the spins suggests we can't reasonably guess times anymore. I am not fussed about this and it seems fine either way; it is just that I have some doubts about what the correct/expected behaviour should be.

  • Right now, Import Playlist is timestamping the playlist if it is imported within the window of the show. There is at least one DJ who does this sometimes. The result is that the moment of import is getting timestamped onto every spin in the playlist. This is a somewhat rare edge case, but it does happen. One suggestion is the following: Keep the time test logic in insertTrack the same, but add a new parameter to insertTrack (both on the implementation and the prototype class IPlaylist) to force automatic timestamps off. This parameter would override timestamping, so that even if insertTrack's time window test would otherwise succeed, no timestamp would be applied. There is only one vector where timestamps are applicable (ui/Playlists.php and any possible future JSON API playlist code in support of Angular/client-side playlist editing). Those invocations could set the parameter to apply the automatic timestamp logic.

Thank you again for this contribution, and for your suggestions to unify the branches and to enhance the installation instructions. Your contributions are making zookeeper even better!

eric-gilbertson pushed a commit to eric-gilbertson/zookeeper that referenced this pull request May 31, 2019
change from self:: to $this-> to invoke non-statics according to PHP convention.
RocketMan pushed a commit that referenced this pull request May 31, 2019
* #32 - playlist timestamping fix & enhancements

   - don't set timetamp when importing a playlist
   - set timestamp on track update if created is null
   - show timestamp for set divider
   - update timestamps when tracks are moved up/down in playlist
   - fix error when clicking down on bottom track

* #32 - don't add timestamp when editing an old playlist

add time window check when updating a playlist check so that
change is not timestamped when editing an older show, eg update
must be done within the the playlist's start/end time in order to be
time stamped.

* #25: use $this to invoke non-static methods

change from self:: to $this-> to invoke non-statics according to PHP convention.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants